From 18a3ef391121d7c4d819448c929721fd1708b40b Mon Sep 17 00:00:00 2001
From: Thomas Reitmayr <treitmayr@devbase.at>
Date: Sun, 27 Oct 2019 21:41:03 +0100
Subject: [PATCH] Fix code generated for Ruby global variables

This commit fixes swig#1653 by creating a Ruby virtual variable
for a C/c++ global variable when SWIG is invoked with the
-globalmodule option.
---
 Doc/Manual/Ruby.html                          | 18 +++++++
 Examples/test-suite/common.mk                 |  2 +
 Examples/test-suite/global_immutable_vars.i   | 24 +++++++++
 .../test-suite/global_immutable_vars_cpp.i    | 24 +++++++++
 Examples/test-suite/ruby/Makefile.in          |  4 ++
 .../ruby/global_immutable_vars_cpp_runme.rb   | 47 +++++++++++++++++
 .../ruby/global_immutable_vars_runme.rb       | 51 +++++++++++++++++++
 .../ruby_global_immutable_vars_cpp_runme.rb   | 47 +++++++++++++++++
 .../ruby/ruby_global_immutable_vars_runme.rb  | 51 +++++++++++++++++++
 .../test-suite/ruby_global_immutable_vars.i   | 25 +++++++++
 .../ruby_global_immutable_vars_cpp.i          | 23 +++++++++
 Source/Modules/ruby.cxx                       | 36 +++++++++----
 12 files changed, 342 insertions(+), 10 deletions(-)
 create mode 100644 Examples/test-suite/global_immutable_vars.i
 create mode 100644 Examples/test-suite/global_immutable_vars_cpp.i
 create mode 100644 Examples/test-suite/ruby/global_immutable_vars_cpp_runme.rb
 create mode 100644 Examples/test-suite/ruby/global_immutable_vars_runme.rb
 create mode 100644 Examples/test-suite/ruby/ruby_global_immutable_vars_cpp_runme.rb
 create mode 100644 Examples/test-suite/ruby/ruby_global_immutable_vars_runme.rb
 create mode 100644 Examples/test-suite/ruby_global_immutable_vars.i
 create mode 100644 Examples/test-suite/ruby_global_immutable_vars_cpp.i

diff --git a/Doc/Manual/Ruby.html b/Doc/Manual/Ruby.html
index 3cfd1292ca..6939a8a186 100644
--- a/Doc/Manual/Ruby.html
+++ b/Doc/Manual/Ruby.html
@@ -615,6 +615,24 @@ <H3><a name="Ruby_nn14">34.3.3 Variable Linking</a></H3>
 effect until it is explicitly disabled using <tt>%mutable</tt>.
 </p>
 
+<p>Note: When SWIG is invoked with the <tt>-globalmodule</tt> option in
+effect, the C/C++ global variables will be translated into Ruby global
+variables. Type-checking and the optional read-only characteristic are
+available in the same way as described above. However the example would
+then have to be modified and executed in the following way:
+
+<div class="code targetlang">
+<pre>$ <b>irb</b>
+irb(main):001:0&gt; <b>require 'Example'</b>
+true
+irb(main):002:0&gt; <b>$variable1 = 2</b>
+2
+irb(main):003:0&gt; <b>$Variable2 = 4 * 10.3</b>
+41.2
+irb(main):004:0&gt; <b>$Variable2</b>
+41.2</pre>
+</div>
+
 <H3><a name="Ruby_nn15">34.3.4 Constants</a></H3>
 
 
diff --git a/Examples/test-suite/common.mk b/Examples/test-suite/common.mk
index 5f77928102..008916a561 100644
--- a/Examples/test-suite/common.mk
+++ b/Examples/test-suite/common.mk
@@ -250,6 +250,7 @@ CPP_TEST_CASES += \
 	funcptr_cpp \
 	functors \
 	fvirtual \
+	global_immutable_vars_cpp \
 	global_namespace \
 	global_ns_arg \
 	global_scope_types \
@@ -689,6 +690,7 @@ C_TEST_CASES += \
 	funcptr \
 	function_typedef \
 	global_functions \
+	global_immutable_vars \
 	immutable_values \
 	inctest \
 	infinity \
diff --git a/Examples/test-suite/global_immutable_vars.i b/Examples/test-suite/global_immutable_vars.i
new file mode 100644
index 0000000000..cd8cb184ba
--- /dev/null
+++ b/Examples/test-suite/global_immutable_vars.i
@@ -0,0 +1,24 @@
+%module global_immutable_vars
+
+// Test immutable and mutable global variables,
+// see http://www.swig.org/Doc4.0/SWIGDocumentation.html#SWIG_readonly_variables
+
+%inline %{
+  int default_mutable_var = 40;
+%}
+
+%immutable;
+%feature("immutable", "0") specific_mutable_var;
+
+%inline %{
+  int global_immutable_var = 41;
+  int specific_mutable_var = 42;
+%}
+
+%mutable;
+%immutable specific_immutable_var;
+%inline %{
+  int global_mutable_var = 43;
+  int specific_immutable_var = 44;
+%}
+
diff --git a/Examples/test-suite/global_immutable_vars_cpp.i b/Examples/test-suite/global_immutable_vars_cpp.i
new file mode 100644
index 0000000000..66eb8545d2
--- /dev/null
+++ b/Examples/test-suite/global_immutable_vars_cpp.i
@@ -0,0 +1,24 @@
+%module global_immutable_vars_cpp
+
+// Test immutable and mutable global variables,
+// see http://www.swig.org/Doc4.0/SWIGDocumentation.html#SWIG_readonly_variables
+
+%inline %{
+  int default_mutable_var = 40;
+%}
+
+%immutable;
+%feature("immutable", "0") specific_mutable_var;
+
+%inline %{
+  int global_immutable_var = 41;
+  int specific_mutable_var = 42;
+%}
+
+%mutable;
+%immutable specific_immutable_var;
+%inline %{
+  int global_mutable_var = 43;
+  int specific_immutable_var = 44;
+%}
+
diff --git a/Examples/test-suite/ruby/Makefile.in b/Examples/test-suite/ruby/Makefile.in
index d75cdb0587..2c59029ec0 100644
--- a/Examples/test-suite/ruby/Makefile.in
+++ b/Examples/test-suite/ruby/Makefile.in
@@ -23,6 +23,7 @@ CPP_TEST_CASES = \
 	li_std_wstring_inherit \
 	primitive_types \
 	ruby_alias_method \
+	ruby_global_immutable_vars_cpp \
 	ruby_keywords \
 	ruby_minherit_shared_ptr \
 	ruby_naming \
@@ -48,6 +49,7 @@ C_TEST_CASES += \
 	li_cstring \
 	ruby_alias_global_function \
 	ruby_alias_module_function \
+	ruby_global_immutable_vars \
 	ruby_manual_proxy \
 
 include $(srcdir)/../common.mk
@@ -57,6 +59,8 @@ SWIGOPT += -w801 -noautorename -features autodoc=4
 
 # Custom tests - tests with additional commandline options
 ruby_alias_global_function.ctest: SWIGOPT += -globalmodule
+ruby_global_immutable_vars.ctest: SWIGOPT += -globalmodule
+ruby_global_immutable_vars_cpp.cpptest: SWIGOPT += -globalmodule
 ruby_naming.cpptest: SWIGOPT += -autorename
 
 # Rules for the different types of tests
diff --git a/Examples/test-suite/ruby/global_immutable_vars_cpp_runme.rb b/Examples/test-suite/ruby/global_immutable_vars_cpp_runme.rb
new file mode 100644
index 0000000000..c40896a867
--- /dev/null
+++ b/Examples/test-suite/ruby/global_immutable_vars_cpp_runme.rb
@@ -0,0 +1,47 @@
+#!/usr/bin/env ruby
+#
+# C++ version of global_immutable_vars_runme.rb
+#
+
+require 'swig_assert'
+
+require 'global_immutable_vars_cpp'
+
+# first check if all variables can be read
+swig_assert_each_line( <<EOF )
+Global_immutable_vars_cpp::default_mutable_var == 40
+Global_immutable_vars_cpp::global_immutable_var == 41
+Global_immutable_vars_cpp::specific_mutable_var == 42
+Global_immutable_vars_cpp::global_mutable_var == 43
+Global_immutable_vars_cpp::specific_immutable_var == 44
+EOF
+
+# check that all mutable variables can be modified
+swig_assert_each_line( <<EOF )
+Global_immutable_vars_cpp::default_mutable_var = 80
+Global_immutable_vars_cpp::default_mutable_var == 80
+Global_immutable_vars_cpp::specific_mutable_var = 82
+Global_immutable_vars_cpp::specific_mutable_var == 82
+Global_immutable_vars_cpp::global_mutable_var = 83
+Global_immutable_vars_cpp::global_mutable_var == 83
+EOF
+
+# now check that immutable variables cannot be modified
+had_exception = false
+begin
+  Global_immutable_vars_cpp::global_immutable_var = 81
+rescue NoMethodError => e
+  had_exception = true
+end
+swig_assert(had_exception, nil,
+            "Global_immutable_vars_cpp::global_immutable_var is writable (expected to be immutable)")
+
+had_exception = false
+begin
+  Global_immutable_vars_cpp::specific_immutable_var = 81
+rescue NoMethodError => e
+  had_exception = true
+end
+swig_assert(had_exception, nil,
+            "Global_immutable_vars_cpp::specific_immutable_var is writable (expected to be immutable)")
+
diff --git a/Examples/test-suite/ruby/global_immutable_vars_runme.rb b/Examples/test-suite/ruby/global_immutable_vars_runme.rb
new file mode 100644
index 0000000000..af55cfeb34
--- /dev/null
+++ b/Examples/test-suite/ruby/global_immutable_vars_runme.rb
@@ -0,0 +1,51 @@
+#!/usr/bin/env ruby
+#
+# Here the proper generation of mutable and immutable variables is tested
+# in the target language.
+# Immutable variables do not have "<var>=" methods generated by SWIG,
+# therefore trying to assign these variables shall throw a NoMethodError
+# exception.
+#
+
+require 'swig_assert'
+
+require 'global_immutable_vars'
+
+# first check if all variables can be read
+swig_assert_each_line( <<EOF )
+Global_immutable_vars::default_mutable_var == 40
+Global_immutable_vars::global_immutable_var == 41
+Global_immutable_vars::specific_mutable_var == 42
+Global_immutable_vars::global_mutable_var == 43
+Global_immutable_vars::specific_immutable_var == 44
+EOF
+
+# check that all mutable variables can be modified
+swig_assert_each_line( <<EOF )
+Global_immutable_vars::default_mutable_var = 80
+Global_immutable_vars::default_mutable_var == 80
+Global_immutable_vars::specific_mutable_var = 82
+Global_immutable_vars::specific_mutable_var == 82
+Global_immutable_vars::global_mutable_var = 83
+Global_immutable_vars::global_mutable_var == 83
+EOF
+
+# now check that immutable variables cannot be modified
+had_exception = false
+begin
+  Global_immutable_vars::global_immutable_var = 81
+rescue NoMethodError => e
+  had_exception = true
+end
+swig_assert(had_exception, nil,
+            "Global_immutable_vars::global_immutable_var is writable (expected to be immutable)")
+
+had_exception = false
+begin
+  Global_immutable_vars::specific_immutable_var = 81
+rescue NoMethodError => e
+  had_exception = true
+end
+swig_assert(had_exception, nil,
+            "Global_immutable_vars::specific_immutable_var is writable (expected to be immutable)")
+
diff --git a/Examples/test-suite/ruby/ruby_global_immutable_vars_cpp_runme.rb b/Examples/test-suite/ruby/ruby_global_immutable_vars_cpp_runme.rb
new file mode 100644
index 0000000000..8453254eb3
--- /dev/null
+++ b/Examples/test-suite/ruby/ruby_global_immutable_vars_cpp_runme.rb
@@ -0,0 +1,47 @@
+#!/usr/bin/env ruby
+#
+# C++ version of ruby_global_immutable_vars_runme.rb.
+#
+
+require 'swig_assert'
+
+require 'ruby_global_immutable_vars_cpp'
+
+# first check if all variables can be read
+swig_assert_each_line( <<EOF )
+$default_mutable_var == 40
+$global_immutable_var == 41
+$specific_mutable_var == 42
+$global_mutable_var == 43
+$specific_immutable_var == 44
+EOF
+
+# check that all mutable variables can be modified
+swig_assert_each_line( <<EOF )
+$default_mutable_var = 80
+$default_mutable_var == 80
+$specific_mutable_var = 82
+$specific_mutable_var == 82
+$global_mutable_var = 83
+$global_mutable_var == 83
+EOF
+
+# now check that immutable variables cannot be modified
+had_exception = false
+begin
+  $global_immutable_var = 81
+rescue NameError => e
+  had_exception = true
+end
+swig_assert(had_exception, nil,
+            "$global_immutable_var is writable (expected to be immutable)")
+
+had_exception = false
+begin
+  $specific_immutable_var = 81
+rescue NameError => e
+  had_exception = true
+end
+swig_assert(had_exception, nil,
+            "$specific_immutable_var is writable (expected to be immutable)")
+
diff --git a/Examples/test-suite/ruby/ruby_global_immutable_vars_runme.rb b/Examples/test-suite/ruby/ruby_global_immutable_vars_runme.rb
new file mode 100644
index 0000000000..fda1ccf0f0
--- /dev/null
+++ b/Examples/test-suite/ruby/ruby_global_immutable_vars_runme.rb
@@ -0,0 +1,51 @@
+#!/usr/bin/env ruby
+#
+# This test program is similar to global_immutable_vars_runme.rb
+# with the difference that the global variables to check are also
+# Ruby global variables (SWIG Ruby option "-globalmodule").
+#
+# Immutable global variables shall throw a NameError exception.
+#
+
+require 'swig_assert'
+
+require 'ruby_global_immutable_vars'
+
+# first check if all variables can be read
+swig_assert_each_line( <<EOF )
+$default_mutable_var == 40
+$global_immutable_var == 41
+$specific_mutable_var == 42
+$global_mutable_var == 43
+$specific_immutable_var == 44
+EOF
+
+# check that all mutable variables can be modified
+swig_assert_each_line( <<EOF )
+$default_mutable_var = 80
+$default_mutable_var == 80
+$specific_mutable_var = 82
+$specific_mutable_var == 82
+$global_mutable_var = 83
+$global_mutable_var == 83
+EOF
+
+# now check that immutable variables cannot be modified
+had_exception = false
+begin
+  $global_immutable_var = 81
+rescue NameError => e
+  had_exception = true
+end
+swig_assert(had_exception, nil,
+            "$global_immutable_var is writable (expected to be immutable)")
+
+had_exception = false
+begin
+  $specific_immutable_var = 81
+rescue NameError => e
+  had_exception = true
+end
+swig_assert(had_exception, nil,
+            "$specific_immutable_var is writable (expected to be immutable)")
+
diff --git a/Examples/test-suite/ruby_global_immutable_vars.i b/Examples/test-suite/ruby_global_immutable_vars.i
new file mode 100644
index 0000000000..dc49cd9467
--- /dev/null
+++ b/Examples/test-suite/ruby_global_immutable_vars.i
@@ -0,0 +1,25 @@
+%module ruby_global_immutable_vars
+
+// This copy of global_immutable_vars.i shall be compiled with the
+// SWIG Ruby option "-globalmodule" in order to check the code path
+// for registering global methods (in contrast to module methods).
+
+%inline %{
+  int default_mutable_var = 40;
+%}
+
+%immutable;
+%feature("immutable", "0") specific_mutable_var;
+
+%inline %{
+  int global_immutable_var = 41;
+  int specific_mutable_var = 42;
+%}
+
+%mutable;
+%immutable specific_immutable_var;
+%inline %{
+  int global_mutable_var = 43;
+  int specific_immutable_var = 44;
+%}
+
diff --git a/Examples/test-suite/ruby_global_immutable_vars_cpp.i b/Examples/test-suite/ruby_global_immutable_vars_cpp.i
new file mode 100644
index 0000000000..cf3145e801
--- /dev/null
+++ b/Examples/test-suite/ruby_global_immutable_vars_cpp.i
@@ -0,0 +1,23 @@
+%module ruby_global_immutable_vars_cpp
+
+// C++ version of ruby_global_immutable_vars.i
+
+%inline %{
+  int default_mutable_var = 40;
+%}
+
+%immutable;
+%feature("immutable", "0") specific_mutable_var;
+
+%inline %{
+  int global_immutable_var = 41;
+  int specific_mutable_var = 42;
+%}
+
+%mutable;
+%immutable specific_immutable_var;
+%inline %{
+  int global_mutable_var = 43;
+  int specific_immutable_var = 44;
+%}
+
diff --git a/Source/Modules/ruby.cxx b/Source/Modules/ruby.cxx
index 6a1e16d5da..c8f582679b 100644
--- a/Source/Modules/ruby.cxx
+++ b/Source/Modules/ruby.cxx
@@ -2192,6 +2192,11 @@ class RUBY:public Language {
     String *getfname, *setfname;
     Wrapper *getf, *setf;
 
+    // Determine whether virtual global variables shall be used
+    // which have different getter and setter signatures,
+    // see https://docs.ruby-lang.org/en/2.6.0/extension_rdoc.html#label-Global+Variables+Shared+Between+C+and+Ruby
+    const bool use_virtual_var = (current == NO_CPP && useGlobalModule);
+
     getf = NewWrapper();
     setf = NewWrapper();
 
@@ -2201,7 +2206,7 @@ class RUBY:public Language {
     getfname = Swig_name_wrapper(getname);
     Setattr(n, "wrap:name", getfname);
     Printv(getf->def, "SWIGINTERN VALUE\n", getfname, "(", NIL);
-    Printf(getf->def, "VALUE self");
+    Printf(getf->def, (use_virtual_var) ? "ID id" : "VALUE self");
     Printf(getf->def, ") {");
     Wrapper_add_local(getf, "_val", "VALUE _val");
 
@@ -2235,8 +2240,12 @@ class RUBY:public Language {
       String *setname = Swig_name_set(NSPACE_TODO, iname);
       setfname = Swig_name_wrapper(setname);
       Setattr(n, "wrap:name", setfname);
-      Printv(setf->def, "SWIGINTERN VALUE\n", setfname, "(VALUE self, ", NIL);
-      Printf(setf->def, "VALUE _val) {");
+      Printf(setf->def, "SWIGINTERN ");
+      if (use_virtual_var) {
+        Printv(setf->def, "void\n", setfname, "(VALUE _val, ID id) {", NIL);
+      } else {
+        Printv(setf->def, "VALUE\n", setfname, "(VALUE self, VALUE _val) {", NIL);
+      }
       tm = Swig_typemap_lookup("varin", n, name, 0);
       if (tm) {
 	Replaceall(tm, "$input", "_val");
@@ -2247,9 +2256,14 @@ class RUBY:public Language {
       } else {
 	Swig_warning(WARN_TYPEMAP_VARIN_UNDEF, input_file, line_number, "Unable to set variable of type %s\n", SwigType_str(t, 0));
       }
-      Printv(setf->code, tab4, "return _val;\n", NIL);
-      Printf(setf->code, "fail:\n");
-      Printv(setf->code, tab4, "return Qnil;\n", NIL);
+      if (use_virtual_var) {
+        Printf(setf->code, "fail:\n");
+        Printv(setf->code, tab4, "return;\n", NIL);
+      } else {
+        Printv(setf->code, tab4, "return _val;\n", NIL);
+        Printf(setf->code, "fail:\n");
+        Printv(setf->code, tab4, "return Qnil;\n", NIL);
+      }
       Printf(setf->code, "}\n");
       Wrapper_print(setf, f_wrappers);
       Delete(setname);
@@ -2259,7 +2273,7 @@ class RUBY:public Language {
     if (CPlusPlus) {
       Insert(getfname, 0, "VALUEFUNC(");
       Append(getfname, ")");
-      Insert(setfname, 0, "VALUEFUNC(");
+      Insert(setfname, 0, (use_virtual_var) ? "(void (*)(ANYARGS))(" : "VALUEFUNC(");
       Append(setfname, ")");
     }
 
@@ -2283,9 +2297,11 @@ class RUBY:public Language {
 	  Printv(s, tab4, "rb_define_singleton_method(", modvar, ", \"", iname, "=\", ", setfname, ", 1);\n", NIL);
 	}
       } else {
-	Printv(s, tab4, "rb_define_global_method(\"", iname, "\", ", getfname, ", 0);\n", NIL);
-	if (!GetFlag(n, "feature:immutable")) {
-	  Printv(s, tab4, "rb_define_global_method(\"", iname, "=\", ", setfname, ", 1);\n", NIL);
+	Printv(s, tab4, "rb_define_virtual_variable(\"$", iname, "\", ", getfname, ", ", NIL);
+	if (GetFlag(n, "feature:immutable")) {
+	  Printv(s, tab4, "0);\n", NIL);
+	} else {
+	  Printv(s, tab4, setfname, ");\n", NIL);
 	}
       }
       Printv(f_init, s, NIL);
